"Implement password sign-in option (#1036)" #1380
Conversation
WalkthroughAdds email+password credential signup and login flows front-to-back, wires a NextAuth CredentialsProvider that supports OTP-token and password paths, persists encrypted passwords in users table, and extends OTP verification/resend to handle credential-based flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Client UI
participant API as /api/auth
participant NextAuth as CredentialsProvider
participant DB as Database
Note right of Frontend: Credential login/signup flows (new)
User->>Frontend: Enter email & password
Frontend->>API: POST /signin or /signup (credentials)
API->>NextAuth: authorize(credentials)
NextAuth->>DB: fetch user, lookup verificationTokens
alt OTP token provided
DB-->>NextAuth: token valid
NextAuth->>DB: delete token
NextAuth-->>API: return user (authenticated)
else Password provided
NextAuth->>DB: verify password hash
alt password match
NextAuth-->>API: return user (authenticated)
else mismatch
NextAuth-->>API: error (invalid credentials)
end
end
API-->>Frontend: success / error
alt needs verification
Frontend->>Frontend: redirect to /verify-otp?type=credentials
end
sequenceDiagram
participant Frontend as Signup UI
participant SignupAPI as /api/auth/signup
participant DB as Database
participant Verify as Verify OTP Page
User->>Frontend: Submit email + password (credential signup)
Frontend->>SignupAPI: POST email, password
SignupAPI->>DB: create user (store encrypted password)
DB-->>SignupAPI: user created
SignupAPI-->>Frontend: success
Frontend->>sessionStorage: store temp password
Frontend->>Verify: redirect to /verify-otp?type=credentials
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
packages/database/auth/auth-options.ts (1)
141-141: Add space after comma for consistency.The code violates formatting conventions.
Apply this diff:
- const isValid = await verifyPassword(user.password,credentials.password); + const isValid = await verifyPassword(user.password, credentials.password);packages/database/auth/drizzle-adapter.ts (1)
41-41: Add space after colon for consistency.Apply this diff:
- password:hashedPassword, + password: hashedPassword,apps/web/app/(org)/signup/form.tsx (2)
44-44: Add space after comma for consistency.As per coding guidelines, use a 2-space indent and consistent spacing.
Apply this diff:
- const [showCredientialLogin,setShowCredientialLogin]=useState(false); + const [showCredientialLogin, setShowCredientialLogin] = useState(false);
259-262: Strengthen password validation requirements.The current validation only checks for a minimum length of 8 characters. Consider adding complexity requirements to improve security:
- At least one uppercase letter
- At least one lowercase letter
- At least one number
- At least one special character
This helps protect against weak passwords and brute-force attacks.
apps/web/app/(org)/login/form.tsx (1)
44-44: Add space after comma for consistency.As per coding guidelines, use a 2-space indent and consistent spacing.
Apply this diff:
- const [showCredientialLogin,setShowCredientialLogin]=useState(false); + const [showCredientialLogin, setShowCredientialLogin] = useState(false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/app/(org)/login/form.tsx(8 hunks)apps/web/app/(org)/signup/form.tsx(8 hunks)apps/web/app/(org)/verify-otp/form.tsx(3 hunks)apps/web/app/(org)/verify-otp/page.tsx(3 hunks)packages/database/auth/auth-options.ts(2 hunks)packages/database/auth/drizzle-adapter.ts(2 hunks)packages/database/schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/database/auth/drizzle-adapter.tspackages/database/auth/auth-options.tsapps/web/app/(org)/verify-otp/form.tsxapps/web/app/(org)/verify-otp/page.tsxapps/web/app/(org)/login/form.tsxpackages/database/schema.tsapps/web/app/(org)/signup/form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/database/auth/drizzle-adapter.tspackages/database/auth/auth-options.tsapps/web/app/(org)/verify-otp/form.tsxapps/web/app/(org)/verify-otp/page.tsxapps/web/app/(org)/login/form.tsxpackages/database/schema.tsapps/web/app/(org)/signup/form.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/verify-otp/form.tsxapps/web/app/(org)/verify-otp/page.tsxapps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/form.tsx
🧬 Code graph analysis (3)
packages/database/auth/auth-options.ts (3)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (1)
users(61-121)packages/database/crypto.ts (1)
verifyPassword(167-197)
apps/web/app/(org)/verify-otp/form.tsx (1)
packages/database/auth/auth-options.ts (1)
signIn(172-202)
apps/web/app/(org)/login/form.tsx (1)
packages/database/auth/auth-options.ts (1)
signIn(172-202)
🔇 Additional comments (2)
apps/web/app/(org)/verify-otp/page.tsx (1)
11-23: LGTM!The type parameter handling is clean and appropriately defaults to "email" when not specified or when the value is not "credentials".
packages/database/schema.ts (1)
55-59: Verify that password encryption happens at the application layer.The custom type
encryptedTextNullableis named to suggest encryption, but it only defines the SQL column type as "text" and does not perform any encryption. Ensure that passwords are hashed (e.g., using PBKDF2, bcrypt, or Argon2) before being passed to the database layer.Run the following script to verify that passwords are hashed before storage:
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apps/web/app/(org)/login/form.tsx (1)
279-294: Fix typo in thetypequery param.The verify-otp flow expects
type=credentials. With the misspelling (credientials), credential logins will land on the OTP page in the wrong mode and fail.- router.push(`/verify-otp?email=${encodeURIComponent(email)}&type=credientials`); + router.push(`/verify-otp?email=${encodeURIComponent(email)}&type=credentials`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(org)/login/form.tsx(8 hunks)apps/web/app/(org)/signup/form.tsx(8 hunks)apps/web/app/(org)/verify-otp/form.tsx(3 hunks)packages/database/auth/auth-options.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(org)/verify-otp/form.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/login/form.tsxpackages/database/auth/auth-options.tsapps/web/app/(org)/signup/form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/login/form.tsxpackages/database/auth/auth-options.tsapps/web/app/(org)/signup/form.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/form.tsx
🧬 Code graph analysis (2)
apps/web/app/(org)/login/form.tsx (1)
packages/database/auth/auth-options.ts (1)
signIn(223-253)
packages/database/auth/auth-options.ts (3)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (2)
users(61-121)verificationTokens(166-172)packages/database/crypto.ts (1)
verifyPassword(167-197)
| if (!user.emailVerified) { | ||
| throw new Error("Please verify your email before logging in."); | ||
| } | ||
|
|
||
| // Delete the one-time token after use | ||
| await db() | ||
| .delete(verificationTokens) | ||
| .where(eq(verificationTokens.identifier, authTokenIdentifier)); | ||
|
|
There was a problem hiding this comment.
OTP branch never completes for new password signups.
For credential signups users.emailVerified is still null when the user submits their OTP. This block throws before you persist the verification, so every new signup through the password flow fails. Instead of rejecting, mark the email as verified when the token matches and only then return the user.
- if (!user.emailVerified) {
- throw new Error("Please verify your email before logging in.");
- }
+ if (!user.emailVerified) {
+ await db()
+ .update(users)
+ .set({ emailVerified: new Date() })
+ .where(eq(users.id, user.id));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!user.emailVerified) { | |
| throw new Error("Please verify your email before logging in."); | |
| } | |
| // Delete the one-time token after use | |
| await db() | |
| .delete(verificationTokens) | |
| .where(eq(verificationTokens.identifier, authTokenIdentifier)); | |
| if (!user.emailVerified) { | |
| await db() | |
| .update(users) | |
| .set({ emailVerified: new Date() }) | |
| .where(eq(users.id, user.id)); | |
| } | |
| // Delete the one-time token after use | |
| await db() | |
| .delete(verificationTokens) | |
| .where(eq(verificationTokens.identifier, authTokenIdentifier)); |
🤖 Prompt for AI Agents
In packages/database/auth/auth-options.ts around lines 158 to 166, the current
code throws if user.emailVerified is falsy which breaks OTP flow for new
credential signups; instead, when the OTP/token matches, update the user's
record to set emailVerified = true (persist the change to the users table), then
delete the one-time verification token and continue to return the user (do not
throw). Ensure the update happens before returning and that the deletion of the
token still occurs after successful verification.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/database/auth/auth-options.ts (1)
158-160: OTP verification blocks new credential signups.For new credential signups,
user.emailVerifiedisnulluntil the user completes OTP verification. This code throws before persisting the verification, creating a circular dependency where users cannot verify their email because they're blocked for not having a verified email.Instead of throwing, update
emailVerifiedwhen the OTP token is valid:- if (!user.emailVerified) { - throw new Error("Please verify your email before logging in."); - } + if (!user.emailVerified) { + await db() + .update(users) + .set({ emailVerified: new Date() }) + .where(eq(users.id, user.id)); + }apps/web/app/(org)/signup/form.tsx (1)
502-515: Remove duplicate autoFocus from password field.Both the email input (line 491) and password input (line 505) have
autoFocus, which causes the password field to steal focus when switching to credential signup, preventing users from immediately typing their email.Apply this diff:
<MotionInput id="password" name="password" - autoFocus type="password" placeholder={emailSent ? "" : "password"} autoComplete="new-password"apps/web/app/(org)/login/form.tsx (1)
495-508: Remove duplicate autoFocus from password field.Both the email input (line 484) and password input (line 498) have
autoFocus, which causes the password field to steal focus when switching to credential login.Apply this diff:
<MotionInput id="password" name="password" - autoFocus type="password" placeholder={emailSent ? "" : "password"} autoComplete="current-password"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(org)/login/form.tsx(8 hunks)apps/web/app/(org)/signup/form.tsx(8 hunks)packages/database/auth/auth-options.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/signup/form.tsxapps/web/app/(org)/login/form.tsxpackages/database/auth/auth-options.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/signup/form.tsxapps/web/app/(org)/login/form.tsxpackages/database/auth/auth-options.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/signup/form.tsxapps/web/app/(org)/login/form.tsx
🧬 Code graph analysis (2)
apps/web/app/(org)/login/form.tsx (2)
packages/database/auth/auth-options.ts (1)
signIn(219-249)packages/database/emails/config.ts (1)
resend(5-6)
packages/database/auth/auth-options.ts (3)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (2)
users(61-121)verificationTokens(166-172)packages/database/crypto.ts (1)
verifyPassword(167-197)
🔇 Additional comments (9)
packages/database/auth/auth-options.ts (4)
4-4: LGTM!The new imports (
eq,and,CredentialsProvider,verificationTokens,verifyPassword) are all necessary for the credential-based authentication flow.Also applies to: 12-12, 17-17, 20-20
110-129: LGTM!The CredentialsProvider configuration and initial validation logic are correct. The email validation and user lookup are properly implemented, and the error message at line 129 is appropriately generic to avoid user enumeration.
175-196: LGTM!The password authentication path correctly validates credentials, requires email verification, and uses generic error messages to prevent user enumeration. The client-side handling of unverified emails (mentioned in the comment at line 180) provides a smooth UX for users who haven't completed verification.
197-201: LGTM!Error handling appropriately logs failures for debugging while re-throwing to allow NextAuth to handle the response.
apps/web/app/(org)/signup/form.tsx (3)
253-297: LGTM!The credential signup flow correctly validates email and password, tracks analytics, calls the signup API, and redirects to OTP verification. The previous security concern about storing passwords in
sessionStoragehas been addressed.
360-381: LGTM!The conditional rendering cleanly switches between credential-based and OTP-based signup flows, with all necessary props correctly passed to each component.
586-596: LGTM!The "Sign up with Password" button is well-integrated and provides a clear pathway for users to switch to credential-based signup.
apps/web/app/(org)/login/form.tsx (2)
368-389: LGTM!The conditional rendering cleanly switches between credential-based and OTP-based login flows with all necessary props correctly passed.
614-624: LGTM!The "Login with Password" button is well-integrated and provides a clear pathway for users to switch to credential-based login.
| null, | ||
| ); | ||
| const [password, setPassword] = useState(""); | ||
| const [showCredientialLogin,setShowCredientialLogin]=useState(false); |
There was a problem hiding this comment.
Fix typo and formatting.
The variable name has a typo ("Crediential" should be "Credential") and is missing spaces around operators.
Apply this diff:
- const [showCredientialLogin,setShowCredientialLogin]=useState(false);
+ const [showCredentialLogin, setShowCredentialLogin] = useState(false);Note: This change will require updating all references to these identifiers throughout the file (lines 253, 368, 376, 387, 467, 475, 522, 553, 562, 619).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/(org)/login/form.tsx around line 44, the state variable
declaration contains a typo and formatting issues: rename showCredientialLogin
to showCredentialLogin, and rename setShowCredientialLogin to
setShowCredentialLogin, adding spaces around the = and after commas; then update
all usages of these identifiers throughout the file (lines 253, 368, 376, 387,
467, 475, 522, 553, 562, 619) to the corrected names so references remain
consistent.
| const resend = await fetch("/api/auth/resend", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ email }), | ||
| }); | ||
|
|
||
| const data = await resend.json(); | ||
| if (!data?.status) throw new Error("Something went wrong, try other login methods."); | ||
|
|
||
| router.push(`/verify-otp?email=${encodeURIComponent(email)}&type=credentials`); | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the /api/auth/resend endpoint exists.
The code calls /api/auth/resend to resend verification during login for unverified accounts. A previous review noted this endpoint was missing from the API contract. Ensure this endpoint is implemented and handles credential-based OTP resends correctly.
Run the following script to check if the endpoint exists:
🏁 Script executed:
#!/bin/bash
# Search for the resend endpoint in API routes and contract
rg -l 'auth.*resend|resend.*auth' --type ts --type tsx -g '!node_modules'
# Check if endpoint is defined in API contract
cat packages/web-domain/src/Http/Api.ts 2>/dev/null | grep -i resend || echo "Not found in API contract"
# Look for Next.js API route handlers
fd 'route\.(ts|js)' apps/web/app/api | xargs rg -l 'resend' 2>/dev/null || echo "No API route found"Length of output: 72
---
🏁 Script executed:
```shell
#!/bin/bash
# Broader search without type restrictions
echo "=== Searching for 'resend' in all files ==="
rg 'resend' -g '!node_modules' -g '!.next' -g '!dist' | head -30
echo -e "\n=== Checking API routes structure ==="
fd -e ts -e js 'app/api' apps/web/ 2>/dev/null | head -20
echo -e "\n=== Looking for auth endpoints ==="
fd -e ts -e js . apps/web/app/api 2>/dev/null | xargs grep -l 'auth' 2>/dev/null | head -10
echo -e "\n=== Checking POST route pattern in api/auth ==="
find apps/web/app/api -name '*route*' -type f 2>/dev/null | head -15
Length of output: 2185
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the catch-all route handler
echo "=== apps/web/app/api/[[...route]]/route.ts ==="
cat apps/web/app/api/[[...route]]/route.ts 2>/dev/null | head -100
echo -e "\n=== apps/web/app/api/auth/[...nextauth]/route.ts (first 150 lines) ==="
cat apps/web/app/api/auth/\[...\]nextauth\]/route.ts 2>/dev/null | head -150
echo -e "\n=== Searching for 'resend' handler implementations ==="
rg -A5 -B2 'resend' apps/web/app/api -g '!node_modules' 2>/dev/null
Length of output: 574
---
🏁 Script executed:
```shell
#!/bin/bash
# Find web-backend package
echo "=== Looking for web-backend package ==="
find . -name "web-backend" -type d -not -path "*/node_modules/*" 2>/dev/null
echo -e "\n=== Searching for HttpLive export ==="
rg 'export.*HttpLive' -g '!node_modules' 2>/dev/null | head -5
echo -e "\n=== Searching for auth/resend in web-backend or backend packages ==="
find . -path "*/packages/*backend*" -name "*.ts" -type f -not -path "*/node_modules/*" 2>/dev/null | xargs rg 'resend|auth.*resend' 2>/dev/null | head -20
echo -e "\n=== Looking for Api.ts or similar contracts ==="
find . -name "Api.ts" -o -name "api.ts" -not -path "*/node_modules/*" 2>/dev/null | head -10
Length of output: 528
| null, | ||
| ); | ||
| const [password, setPassword] = useState(""); | ||
| const [showCredientialLogin,setShowCredientialLogin]=useState(false); |
There was a problem hiding this comment.
Fix typo and formatting.
The variable name has a typo ("Crediential" should be "Credential") and is missing spaces around operators.
Apply this diff:
- const [showCredientialLogin,setShowCredientialLogin]=useState(false);
+ const [showCredentialLogin, setShowCredentialLogin] = useState(false);Note: This change will require updating all references to these identifiers throughout the file (lines 253, 360, 368, 379, 474, 482, 522, 529, 548, 557, 591).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around line 44, the state variable is
misspelled and poorly formatted: change const
[showCredientialLogin,setShowCredientialLogin]=useState(false); to use a
correctly spelled and spaced identifier (e.g., showCredentialLogin and
setShowCredentialLogin) and add spaces around operators; then update every
reference to the old identifiers throughout this file at lines 253, 360, 368,
379, 474, 482, 522, 529, 548, 557, and 591 to use the new names so
imports/usages remain consistent.
1 participant
@richiemcilroy can you please review this ? I have tested this and it works fine. let me know if any changes required
Summary by CodeRabbit